Support REFRESH MATERIALIZED VIEW by gggrace14 · Pull Request #15996 · prestodb/presto

gggrace14 · 2021-04-26T05:08:04Z

Depended by https://github.com/facebookexternal/presto-facebook/pull/1509

highker · 2021-04-30T01:13:28Z

Let's rebase this PR on top of #15910. That one is one a shape that won't have major interface or structure change.

gggrace14 · 2021-04-30T21:59:11Z

Let's rebase this PR on top of #15910. That one is one a shape that won't have major interface or structure change.

I will do it and come back.

highker

Quick comments on the first commit

jainxrohit · 2021-05-10T14:01:39Z

Have only looked at the query building part of it, have left some comments for clarification.

highker

Let's clean up the initial comments, rebase, and have another round of review

jainxrohit · 2021-05-26T05:34:51Z

Can we please resolve the the comments which are already addressed?

jainxrohit

Left the change in comments

gggrace14 · 2021-06-04T04:27:33Z

Addressed comments, and also changed TestHiveLogicalPlanner to use REFRESH MV to write to MVs and enabled the test cases.

jainxrohit · 2021-06-05T05:02:26Z

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

jainxrohit

Overall it looks good to me. Lets make the final set of requested changes.

I would have preferred to keep the query rewriting logic at one place, as it is easier to read and debug. But may be we can create a task for refactoring it.

cc: @highker

gggrace14 · 2021-06-07T22:44:06Z

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

Yes, testMaterializedViewForJoinWithMultiplePartitions is enabled now and it is good with replacing insert w/ refresh. Fixing testMaterializedViewForJoin needs more complete population of the columnMappings structure, which is orthogonal to REFRESH and deserves a standalone PR. Filing the issue #16220 and updating the TODO.

jainxrohit · 2021-06-07T22:53:13Z

testMaterializedViewForJoinWithMultiplePartitions

I was also talking about test method
public void testMaterializedViewForJoin()

gggrace14 · 2021-06-08T20:19:18Z

@gggrace14 are we planning to fix testMaterializedViewForJoin test as part of this PR?
If not can we create a new task and update the TODO?

I see that testMaterializedViewForJoinWithMultiplePartitions as well is disabled. Can we enable it if there are no issues?

Yes, testMaterializedViewForJoinWithMultiplePartitions is enabled now and it is good with replacing insert w/ refresh.

gggrace14 · 2021-06-08T20:35:55Z

testMaterializedViewForJoinWithMultiplePartitions

I was also talking about test method
public void testMaterializedViewForJoin()

Filing the issue #16220 and updating the TODO.

highker · 2021-06-13T06:12:16Z

Remove this interface. Let's only keep the constructor(s)

highker · 2021-06-13T06:13:21Z

Use the constructor instead of withXXX

highker · 2021-06-13T06:13:43Z

No need to call it viewDefinitionWithRefreshColumns, just reuse viewDefinition

highker · 2021-06-13T06:33:18Z

merge into one line

highker · 2021-06-13T06:33:35Z

typo: REFERSH

highker · 2021-06-13T06:35:19Z

Let's keep these unchanged

ajaygeorge · 2021-06-16T04:27:25Z

@gggrace14 There are no release notes on this PR. Can you please add them.
cc @highker / @jainxrohit

jainxrohit · 2021-06-16T04:28:50Z

@gggrace14 There are no release notes on this PR. Can you please add them.
cc @highker / @jainxrohit

My bad, @gggrace14, let add no release notes for this PR.

gggrace14 · 2021-06-16T05:06:31Z

Yes, will do!

gggrace14 force-pushed the mra branch from 0277648 to 52008e7 Compare April 26, 2021 06:11

gggrace14 requested review from highker and jainxrohit April 26, 2021 07:28

gggrace14 marked this pull request as ready for review April 26, 2021 07:29

highker requested a review from kaikalur April 26, 2021 17:40

gggrace14 force-pushed the mra branch 3 times, most recently from ce11808 to 3579314 Compare May 3, 2021 17:32

jainxrohit added the materialized-view label May 3, 2021

gggrace14 force-pushed the mra branch 3 times, most recently from 984033e to 3794b30 Compare May 3, 2021 22:19

highker reviewed May 4, 2021

View reviewed changes

gggrace14 requested a review from highker May 4, 2021 08:04

jainxrohit reviewed May 6, 2021

View reviewed changes

Comment thread presto-main/src/main/java/com/facebook/presto/sql/analyzer/Analysis.java Outdated

jainxrohit requested changes May 10, 2021

View reviewed changes

Comment thread presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java Outdated

Comment thread presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java Outdated

highker reviewed May 18, 2021

View reviewed changes

gggrace14 force-pushed the mra branch 4 times, most recently from 7a0c4b7 to b141d3d Compare May 20, 2021 18:04

gggrace14 requested review from highker and jainxrohit May 20, 2021 21:51

jainxrohit requested changes May 24, 2021

View reviewed changes

Comment thread presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java Outdated

Comment thread presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java Outdated

Comment thread presto-hive/src/main/java/com/facebook/presto/hive/HiveWriterFactory.java Outdated

jainxrohit requested changes May 26, 2021

View reviewed changes

jainxrohit requested changes May 27, 2021

View reviewed changes

jainxrohit requested changes Jun 1, 2021

View reviewed changes

gggrace14 force-pushed the mra branch from 49fbaa0 to 95037c2 Compare June 4, 2021 00:54

gggrace14 commented Jun 4, 2021

View reviewed changes

Comment thread presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java Outdated

gggrace14 force-pushed the mra branch from 95037c2 to 6ef353e Compare June 4, 2021 04:22

jainxrohit requested changes Jun 5, 2021

View reviewed changes

Comment thread presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java Outdated

Comment thread presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java Outdated

gggrace14 force-pushed the mra branch from 6ef353e to a52e1da Compare June 7, 2021 22:45

gggrace14 force-pushed the mra branch from a52e1da to 860c9a2 Compare June 7, 2021 23:04

jainxrohit requested changes Jun 7, 2021

View reviewed changes

Comment thread presto-hive/src/test/java/com/facebook/presto/hive/TestHiveLogicalPlanner.java Outdated

jainxrohit approved these changes Jun 8, 2021

View reviewed changes

gggrace14 force-pushed the mra branch from 860c9a2 to 546e4d0 Compare June 8, 2021 23:11

highker approved these changes Jun 13, 2021

View reviewed changes

highker self-assigned this Jun 13, 2021

gggrace14 force-pushed the mra branch 3 times, most recently from 6b7d0db to 17bd4f4 Compare June 14, 2021 16:29

gggrace14 added 2 commits June 14, 2021 13:39

Support syntax and analyze for REFRESH MATERIALIZED VIEW

f66f410

Support REFRESH MATERIALIZED VIEW execution

a58bda7

gggrace14 force-pushed the mra branch from 17bd4f4 to a58bda7 Compare June 14, 2021 20:40

highker merged commit f8f2690 into prestodb:master Jun 15, 2021

mayankgarg1990 mentioned this pull request Jun 18, 2021

Add release notes for 0.256 #16296

Merged

2 tasks

Conversation

gggrace14 commented Apr 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highker commented Apr 30, 2021

Uh oh!

gggrace14 commented Apr 30, 2021

Uh oh!

highker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jainxrohit commented May 10, 2021

Uh oh!

Uh oh!

Uh oh!

highker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jainxrohit commented May 26, 2021

Uh oh!

jainxrohit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gggrace14 commented Jun 4, 2021

Uh oh!

jainxrohit commented Jun 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jainxrohit left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gggrace14 commented Jun 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jainxrohit commented Jun 7, 2021

Uh oh!

Uh oh!

gggrace14 commented Jun 8, 2021

Uh oh!

gggrace14 commented Jun 8, 2021

Uh oh!

highker Jun 13, 2021

Choose a reason for hiding this comment

Uh oh!

highker Jun 13, 2021

Choose a reason for hiding this comment

Uh oh!

highker Jun 13, 2021

Choose a reason for hiding this comment

Uh oh!

highker Jun 13, 2021

Choose a reason for hiding this comment

Uh oh!

gggrace14 commented Apr 26, 2021 •

edited

Loading

jainxrohit commented Jun 5, 2021 •

edited

Loading

jainxrohit left a comment •

edited

Loading

gggrace14 commented Jun 7, 2021 •

edited

Loading